Prevent deadlock on thread synchronization via CountDownLatch #1538
Conversation
ljackiewicz
force-pushed
the
ljackiewicz/prevent-deadlock-on-thead-sync
branch
from
dd56edd
to
8d819ba
Compare
There was a problem hiding this comment.
Does not smell right. IIRC a sh step will normally “finish” almost instantly because it is really just forking a process and returning, so that should not be affected by a 1m timeout, but “non-durable” steps like checkout or junit etc. might legitimately run for longer.
It is hard to say from Jira what the real issue is because the line numbers do not seem to match the current version of the pipeline-maven plugin. I am guessing this is https://github.com/jenkinsci/pipeline-maven-plugin/blob/8cfaff9c021c971d19e5469c553a86d954c05387/pipeline-maven/src/main/java/org/jenkinsci/plugins/pipeline/maven/WithMavenStepExecution2.java#L573 which ought not be hanging.
withMaven does tricky stuff and I do not recommend mixing it with the container step generally; it is more straightforward to just run sh 'mvn …'; junit '**/target/surefire-reports/TEST-*.xml'.
|
Okay, I wasn't entirely aware of the impact on non-durable steps. Referring to the mentioned issue, when calling withMaven step, in my case it looks like the process hangs randomly when calling the join method on the Proc object in the WithMavenStepExecution2.readFromProcess() method (https://github.com/jenkinsci/pipeline-maven-plugin/blob/8cfaff9c021c971d19e5469c553a86d954c05387/pipeline-maven/src/main/java/org/jenkinsci/plugins/pipeline/maven/WithMavenStepExecution2.java#L685). I'm not sure what exactly this problem is related to, but having looked through the ContainerExecProc implementation, I don't quite understand what the reason for using CountDownLatch was, which in this case, at least indirectly, may be causing the problem by not protecting against possible deadlocks. We have considered discontinuing the use of withMaven step (although a similar problem also occurs when calling other steps), but the functionality of triggering dependent projects is significant to us. |
This PR resolves reported issue JENKINS-72792 by preventing deadlock on thread synchronization via CountDownLatch.
The changes were tested on an instance running various types of pipelines. No negative impact was noticed.
In case a deadlock occurs while executing a withMaven step (case described in the reported issue), a one-minute timeout is visible.